Modify post_fp_cp2k, adapt to SCF not converged situation.
#1562
Conversation
… to run different system at one single iteration model_devi step.
WalkthroughWalkthroughThe changes to the Changes
Sequence Diagram(s)Old FlowsequenceDiagram
participant Client
participant post_fp_cp2k
Client->>+post_fp_cp2k: Call function with iter_index, jdata, rfailed
post_fp_cp2k->>post_fp_cp2k: Initialize all_sys
loop sys_output
post_fp_cp2k->>post_fp_cp2k: Process system
end
post_fp_cp2k->>-Client: Return result
New FlowsequenceDiagram
participant Client
participant post_fp_cp2k
participant LogFile
Client->>+post_fp_cp2k: Call function with iter_index, jdata, rfailed
post_fp_cp2k->>post_fp_cp2k: Initialize all_sys
loop sys_output
post_fp_cp2k->>post_fp_cp2k: Read file content
alt SCF run NOT converged
post_fp_cp2k->>LogFile: Write log entry
post_fp_cp2k->>post_fp_cp2k: Skip file
else
post_fp_cp2k->>post_fp_cp2k: Process system
post_fp_cp2k->>post_fp_cp2k: Append system to all_sys
post_fp_cp2k->>post_fp_cp2k: Increment count
end
end
post_fp_cp2k->>-Client: Return result
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional comments not posted (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
for more information, see https://pre-commit.ci
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #1562 +/- ##
=======================================
Coverage 49.59% 49.60%
=======================================
Files 83 83
Lines 14844 14852 +8
=======================================
+ Hits 7362 7367 +5
- Misses 7482 7485 +3 ☔ View full report in Codecov by Sentry. |
dpgen/generator/run.py
Outdated
| log_file_path = os.path.join(work_path, f"{ss}.fp-fail.log") | ||
| all_sys = dpdata.MultiSystems(type_map=jdata["type_map"]) | ||
| for oo in sys_output: | ||
| _sys = dpdata.LabeledSystem(oo, fmt="cp2k/output") |
There was a problem hiding this comment.
why does this fmt revert back?
Lines 4483 to 4489 in 54e48c6
dpgen/generator/run.py
Outdated
| all_sys.append(_sys) | ||
| with open(oo, 'r') as file: | ||
| content = file.read() | ||
| if 'SCF run NOT converged' in content: |
There was a problem hiding this comment.
cp2kdata already checks the convergence.
if the scf is not converged, dpdata will return empty object.
https://github.com/robinzyb/cp2kdata/blob/d74c14cbf7470451af443c706d8479c8b486a68d/cp2kdata/dpdata_plugin.py#L24-L43
There was a problem hiding this comment.
Yes, it will. But when using MultiSystems, the empty object will be added into that, for example create an empty "C0H0O0". This will raise error when save the MultiSystems to data.xxx. Maybe we should change the subroutine which you mentioned to skip instead of creating empty object.
| with open(oo) as file: | ||
| content = file.read() | ||
| if "SCF run NOT converged" in content: | ||
| with open(log_file_path, "a") as log_file: | ||
| log_file.write(f"Skipping file {oo} due to SCF run NOT converged\n") | ||
| continue |
There was a problem hiding this comment.
move these lines after _sys=dpdata.LabeledSystem
| with open(oo) as file: | |
| content = file.read() | |
| if "SCF run NOT converged" in content: | |
| with open(log_file_path, "a") as log_file: | |
| log_file.write(f"Skipping file {oo} due to SCF run NOT converged\n") | |
| continue | |
| if len(_sys) == 0: | |
| with open(log_file_path, "a") as log_file: | |
| log_file.write(f"Skipping file {oo} due to SCF run NOT converged\n") | |
| continue |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| all_sys.append(_sys) | ||
| icount += 1 | ||
|
|
||
| icount += len(all_sys) |
There was a problem hiding this comment.
Duplicate icount increment
icount is incremented twice. Remove the duplicate increment.
- icount += len(all_sys)Committable suggestion was skipped due to low confidence.
| if "SCF run NOT converged" in content: | ||
| with open(log_file_path, "a") as log_file: | ||
| log_file.write(f"Skipping file {oo} due to SCF run NOT converged\n") | ||
| continue |
There was a problem hiding this comment.
Move SCF check and logging after _sys instantiation
To ensure _sys is only instantiated if SCF converges, move these lines after _sys=dpdata.LabeledSystem.
- if "SCF run NOT converged" in content:
- with open(log_file_path, "a") as log_file:
- log_file.write(f"Skipping file {oo} due to SCF run NOT converged\n")
- continue
_sys = dpdata.LabeledSystem(
oo, fmt="cp2kdata/e_f", type_map=jdata["type_map"]
)
+ if "SCF run NOT converged" in content:
+ with open(log_file_path, "a") as log_file:
+ log_file.write(f"Skipping file {oo} due to SCF run NOT converged\n")
+ continueCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if "SCF run NOT converged" in content: | |
| with open(log_file_path, "a") as log_file: | |
| log_file.write(f"Skipping file {oo} due to SCF run NOT converged\n") | |
| continue | |
| _sys = dpdata.LabeledSystem( | |
| oo, fmt="cp2kdata/e_f", type_map=jdata["type_map"] | |
| ) | |
| if "SCF run NOT converged" in content: | |
| with open(log_file_path, "a") as log_file: | |
| log_file.write(f"Skipping file {oo} due to SCF run NOT converged\n") | |
| continue |
3 participants
Check whether "SCF run NOT converged" appears before reading the output file. Print the path of unfinished calculation to the log file. Make the
icountcounter work effectivelySummary by CodeRabbit